feat(clerk-js,types): Support connecting Coinbase Wallet via <UserProfile /> - #4030
Conversation
🦋 Changeset detectedLatest commit: 96c472c The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
86abc30 to
fb321fd
Compare
panteliselef
left a comment
panteliselef
left a comment
There was a problem hiding this comment.
Minor request about the localization key
| // @ts-ignore | ||
| if (!global.ethereum) { | ||
| // Do nothing when ethereum doesn't exist. We might revise this in the future | ||
| // to offer an Install Metamask prompt to our users. |
There was a problem hiding this comment.
Is this recommendation not viable ? (just curious)
There was a problem hiding this comment.
Not sure if it's possible or what's the recommended way (hopefully we will find out soon), but removed the comment until we have a concrete plan in place
| isLoading={card.loadingMetadata === strategy} | ||
| isDisabled={card.isLoading} | ||
| localizationKey={`Connect ${strategyToDisplayData[strategy].name} wallet`} | ||
| localizationKey={`Connect ${strategyToDisplayData[strategy].name}`} |
There was a problem hiding this comment.
I believe it's time to create a proper localization key here
There was a problem hiding this comment.
I went ahead and aligned this with the Social connections section. What's changed is:
- Update the copyright of the primary button from
Web3 walletstoConnect wallet - Introduced a new localization key
userProfile.web3WalletPage.web3WalletButtonsBlockButtonto only render the web3 provider name on the dropdown
| "@clerk/clerk-js": patch | ||
| "@clerk/types": patch |
There was a problem hiding this comment.
I'd do minor because it affects the User interface
There was a problem hiding this comment.
Updated to use a minor version
alexcarpenter
left a comment
alexcarpenter
left a comment
There was a problem hiding this comment.
Doesn't look like it would be included in this PR, but highlighting that the logo - name spacing is a bit off here. The logo is almost touching the name coinbase.
Can we add a UI ticket to follow up and improve here?
fb321fd to
c499432
Compare
@alexcarpenter it was an easy fix so i went ahead and fixed that as well as part of this PR. That's how it looks right now: |
panteliselef
left a comment
panteliselef
left a comment
There was a problem hiding this comment.
minor comment about the changelog, otherwise looks good
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| "@clerk/clerk-js": minor | |||
| "@clerk/types": minor | |||
There was a problem hiding this comment.
| "@clerk/types": minor | |
| "@clerk/localizations": minor |
c499432 to
657ce8b
Compare
…t via <UserProfile />
657ce8b to
96c472c
Compare
4 participants
Description
Allow users to connect their Coinbase Wallet to their account via
<UserProfile />Screen.Recording.2024-08-27.at.5.35.46.PM.mov
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change